ci: adopt the fleet reusable CI workflow - #5
Merged
Conversation
Replaces this repo's hand-maintained ci.yml with a call to the shared workflow_call workflow. Repo-specific jobs stay here alongside the call. The fleet carried 91 copies of ci.yml in 89 distinct normalized variants across 49 feature profiles. The per-repo copies left four gaps this closes: - Secret scan: 48 of 91 repos had none. - SHA pinning: 10 of 91 were fully pinned; 81 carried a floating tag. - permissions: 0 of 91 declared a block. - Coverage: 6 implementations with 3 incompatible semantics. The shared coverage gate is per-line and honours cov:unreachable, because that is the only semantics that preserves defence in depth. An aggregate floor has no per-line hook, so it cannot honour an exemption and never names the rotting lines; a naive DA:n,0 grep has no exemption at all, so the only way to green is to delete the guard. Both pressure the author toward removing exactly the code that keeps a parser safe on hostile input. The callee does not exist yet -- creating the repo that hosts it is not yet authorized -- so the `ci` check will fail to resolve until it lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous pin predates the rust-cache provenance correction, so a reviewer following the SHA would read the wrong figures (the finding is 56 repos / 87 files / 292 occurrences, not "~36 repos"). Workflow bytes are unchanged; only the README differs between the two commits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts actions/checkout v7.0.1 -> v4.2.2 and rust-cache v2.9.1 -> the genuine v2.7.8 in this repo's own jobs, matching the shared workflow. Every action now pins exactly what the fleet runs today, so adopting the reusable workflow changes one variable -- where CI is defined -- and not two. Repins fleet-config to the commit that fixes the cargo-deny argument order (--config belongs to the `check` subcommand; the previous form would have failed at runtime) and corrects the permissions figure to 22 of 92 repos. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous fleet-config commit still had one checkout on v7.0.1 -- the deny job's `Fetch shared deny.toml` step, whose `uses:` is indented under a `- name:` key and so escaped a whole-file replace. All 25 action refs in the callee are now pinned and each SHA re-verified against its commented tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The caller stub referenced SecurityRonin/fleet-config/.github/workflows/rust-ci.yml, but the reusable workflow lives in SecurityRonin/fleet-ci. fleet-config holds the shared deny.toml and the legal templates and has no workflows directory, so the reference could never have resolved. Also repins from 29f8011 to f49dff5, which carries the fix for a defect the workflow had inherited from the repos it consolidates: a bare "cargo fetch" re-resolves and rewrites Cargo.lock in the runner, so the "cargo vet --locked" on the following line validated a lock CI had just generated for itself rather than the committed bytes. 80 of 81 fleet repos share that shape. Consolidating the workflow as previously pinned would have propagated it to all 91 repos in a single shared file everyone assumes was reviewed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
h4x0r
force-pushed
the
ci/adopt-reusable-workflow
branch
from
August 5, 2026 22:42
84f98ae to
7aa0f9c
Compare
The fleet coverage scope (--workspace --all-features) compiles core/src/vfs.rs,
which the old --lib job never did, and found 37 uncovered lines. They were not
random: the ten integration tests all address the volume correctly, so every
error branch in the adapter was unexercised. This adds nine tests for them.
What is now covered, and why each is worth a test rather than an exclusion:
entry_of / stream_name refusals — FileId is a fleet-wide union where an ext4
inode, an APFS oid and an NTFS reference are all integers in a struct. If this
adapter coerced ExtInode { ino: 5 } into MFT record 5 it would return real
bytes from the wrong object under a request that looks entirely valid. Same
for a named stream silently served as $DATA. Both are tested at the helper
level and again through every trait entry point on a genuinely mounted volume.
map_err — asserts I/O stays distinct from Decode. Collapsing them would let a
failed read masquerade as a corrupt filesystem, which inverts the diagnosis.
namespace_rank — asserted as a whole ordering (WIN32_AND_DOS > WIN32 > POSIX >
DOS) rather than arm by arm, since the ordering is the property that decides
which name an examiner sees.
best_file_name on unparseable records — returns None rather than a fabricated
name.
One test documents behaviour I got wrong on first write, and it is the more
interesting one. I assumed an attribute offset pointing past the end of a record
would make build_meta error. It does not: parse_attributes degrades to an empty
list and build_meta returns Ok. The test now asserts what actually holds — no
panic, and no fabricated facts: every MAC(B) time is None rather than a zeroed
FILETIME, because "1601-01-01" in a timeline is a claim about the evidence that
nothing in the record supports.
Also included is a control test: a valid NtfsRef still resolves on the same
handle. Without it the refusal tests would pass equally well against a volume
that rejected everything.
INCOMPLETE — the gate is not satisfied. 37 uncovered lines are now 27
(93.45% -> 94.18%); workspace total is 99.27%. The remainder needs synthetic
fixtures this repo does not have: a volume with no $Volume record to drive
volume_label's degradation path, and records crafted to make read_record fail
mid-walk for the .map_err(map_err) error arms. That is real work, not a
contrivance, and it is deliberately not faked here — chasing the last lines with
tests written to touch code rather than to assert behaviour is the failure mode
the coverage policy exists to prevent.
…lusion The remaining 27 uncovered lines in core/src/vfs.rs are recorded rather than hidden, and the mechanism is chosen to keep them visible. Three options were on the table: `// cov:unreachable: <invariant>` on each line — REJECTED. That marker asserts a guard is provably dead under a stated invariant. These lines are reachable; they are merely untested. Writing the annotation anyway would be a false statement, and a suppression whose stated reason is wrong is worse than a bare one because it survives review by looking rigorous. fleet-ci even enforces the `: <invariant>` suffix precisely so the assertion is reviewable — inventing an invariant to satisfy it would defeat the check by complying with it. `coverage-ignore-regex: 'core/src/vfs\.rs'` — REJECTED. It disables the gate for all 443 lines of the file to excuse 27, so a later regression in the 416 that ARE covered would go unreported. Excusing a known gap must not also create an unknown one. `coverage-gate: floor` — TAKEN. Every line stays measured, the aggregate is still enforced, and the debt announces itself: fleet-ci renames the check to "Coverage (FLOOR 99% — migration debt, not the fleet gate)" and emits a ::warning:: on every run saying to migrate to strict. A reader of the check list sees the exemption without opening a file. The floor is 99, just under the current 99.27%, so it holds the line rather than granting slack — a real regression still fails. `--fail-under-lines` does the comparison, so the gate can fail. The ci.yml comment names exactly what each remaining branch needs: a volume with no $Volume record, records crafted to fail mid-walk for the .map_err arms, and a malformed $STANDARD_INFORMATION. REMOVE WHEN those fixtures exist — then drop both inputs and inherit the fleet strict gate.
Answers the question these lines actually pose: can they be tested honestly, or
only exempted? They can be tested, and at Tier 2 rather than Tier 3.
The distinction that matters is not synthetic-vs-real. It is whether a test
ASSERTS BEHAVIOUR or merely EXECUTES A LINE. A crafted malformed volume that
asserts "degrades without fabricating" is the former, and the tier rule says so
explicitly: T3 is barred as sole validation for a path that produces a value an
oracle could cross-check (decoders, parsers), while robustness and negative
tests are named as where it is legitimate. These lines are refusal and
degradation branches — they produce errors and absent values, not values an
oracle could check.
Every test here goes further than that and reaches T2: each starts from the same
real partition.dd the suite already validates against TSK, applies ONE mutation
stated in the test, and derives the expected result from that construction
rather than from an answer chosen by the author.
truncation sweep — cut the image at five points. The volume either
refuses to mount or degrades per-record; both are
acceptable, a panic is not.
$Volume corruption — overwrite the "FILE" signature on every copy of MFT
record 3, so no label is readable. The only honest
output is None; a placeholder would be
indistinguishable in a report from a volume named "".
Two controls, because both tests would otherwise pass vacuously:
- `truncation_actually_removes_readable_records` fails if no cut point makes a
previously-readable record unreadable. Without it the sweep could assert
"nothing panicked" over an undamaged volume.
- `the_intact_volume_does_report_a_label` fails if the sample volume has no
label to lose. Without it, `volume_label() == None` proves nothing about the
damage.
The first version of the $Volume test damaged only one copy of record 3 and the
label still resolved — the image carries an $MFTMirr. The offsets are now
derived by scanning for the record-number field rather than hard-coded, which is
what caught it.
core/src/vfs.rs: 37 uncovered lines at adoption, 26 now. The coverage floor
added in the previous commit stays for the remainder; it is not a substitute for
this work, it is what holds the line while the rest of it happens.
`ci / cargo-deny` failed on RUSTSEC-2026-0002 — lru's `IterMut` violating
Stacked Borrows, categorised memory-corruption. Fixed by upgrade rather than
ignored:
lru 0.9.0 -> 0.16.4 (patched = ">= 0.16.3", unaffected = "< 0.9.0")
lru is not a direct dependency. It arrives only through `mft`, which this repo
uses as a DEV-dependency for the differential parity gate in
tests/parity_mft.rs, so nothing an examiner runs ever linked it. That lowers the
severity but does not make it someone else's problem: mft 0.7.0 requires
lru ^0.16.1, which resolves to a patched 0.16.x, and the requirement here was
pinned at "0.6" so `cargo update` alone could never cross it.
TRADE-OFF, stated because it is a judgement rather than a free win: mft 0.7
brings a much larger dev tree — 43 crates new to the vet store (sonic-rs, rkyv,
jiff, defmt and their dependencies). The alternative was keeping mft 0.6 and
recording the advisory as dev-only. Upgrading was chosen because the fleet rule
puts "update to the patched version" first and reaches for suppression only when
the true fix is unavailable, which here it plainly was not. The cost is visible
and reversible: revert this commit and the tree shrinks back.
All 43 are recorded as [[exemptions]] at `safe-to-run`, the criteria for
dev-only code, via `cargo vet regenerate exemptions`. An exemption states
honestly that the code is unaudited. `cargo vet certify --accept-all` was NOT
used: a certify record asserts a human read the source, so bulk-certifying would
fabricate the very condition being claimed. The audits.toml self-certified count
is still zero after this change.
Verified: cargo deny advisories ok, cargo vet --locked succeeds, fmt and clippy
clean, 12 test suites pass including the mft parity gate on the new major.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
h4x0r
marked this pull request as ready for review
August 6, 2026 03:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pilot 3 of 3 — workspace with fuzz (
ntfs-core+ntfs-forensic, rootfuzz/). 172 lines become 44.This one surfaces a real finding. The workflow being replaced runs
cargo llvm-cov --libwith no features; the fleet default is--workspace --all-features. Running the shared gate locally at the fleetdefault surfaces 37 uncovered lines in
core/src/vfs.rs— thevfsfeatureadapter, which the narrower scope never measured at all. The old job is named
"Coverage (100% lines)" and its number was true for what it measured, and silent
about a whole feature-gated module.
Those 37 lines are left visible on purpose. The two honest ways to clear them are
to test the vfs adapter, or to declare the exclusion explicitly with
coverage-ignore-regex: 'core/src/vfs\.rs'— which is at least reviewable.Narrowing scope back to
--libwould hide the gap again. Scope is part of thecoverage semantics, not a detail.
Why
The fleet carries 91 copies of
ci.ymlin 89 distinct normalized variantsacross 49 feature profiles. Where a shared mechanism exists the fleet
converges completely —
release-plz.ymlhas 87 copies in 2 variants, andrust-toolchain.tomlis uniform at 1.96.0 across all 90. CI drifted becausenothing held it together.
Four gaps the single workflow closes:
permissions:blockcontents: read, workflow-wideThe pins are sound; one provenance label is fiction
9bdad043e88c75890e36ad3bbc8d27f0090dd609appears in 56 fleet repos, across 87workflow files, 292 times — 276 commented
# v2.7.8, 16 commented# v2. It isa real
Swatinem/rust-cachecommit dated 2024-05-03 (fix: usage of deprecated version of node, PR #197), but it matches no release tag: v2.7.3 →23bce251a8cd2ffc3c1075eaa2367cf899916d84, v2.7.5 →82a92a6e8fbeee089604da2575dc567ae9ddeaab, v2.7.7 →f0deed1e0edfc6a9be95417288c0e1099b1eeec3, v2.7.8 →9d47c6ad4b02e050fd481d890b2ea34778fd09d6, v2.8.0 →98c8021b550208e191a6a3145459bfc9fb29c4c0.This is a traceability defect, not a vulnerability. A SHA is immutable, so the
security control is doing its job — the pinned bytes cannot change under anyone.
What fails is provenance: 56 repos run untagged mid-tree code while the comment
beside it asserts a release, so an audit answering "are we on released
versions?" from those comments gets a wrong answer, and Renovate's
digest-pinning may not map SHA→version cleanly either.
orchestration/issenandparser/browser-forensicpin both SHAs in differentworkflows, so they are internally inconsistent about which
rust-cachethey run.Every SHA in the shared workflow was resolved with
gh api repos/<owner>/<repo>/commits/<tag> --jq .sha;rust-cacheisc19371144df3bb44fab255c43d04cbc2ab54d1c4, genuinely v2.9.1.The coverage gate, and why these semantics
The fleet's gate existed in three forms that disagree about what "100%" asserts:
cov:unreachable-aware (36 repos) — walks everyDA:<n>,0record, reads the source line, exempts annotated ones, names what failed.
--fail-under-lines N(10 repos, N ∈ {85, 89, 90, 92, 95,96, 97}).
grep -qE '^DA:[0-9]+,0$'— strict 100%, no exemption at all.The shared workflow implements (1). It is what the fleet constitution already
requires, and it is the only form that preserves defence in depth: under a floor,
an unreachable guard is indistinguishable from an untested one; under the naive
grep, the only way to go green is to delete the guard. Both pressure the author
toward removing exactly the code that keeps a parser safe on hostile input. A
floor also hides which lines rot — at 85%, 15% of the crate can decay silently,
and every floor in the fleet carried a "ratchet it up later" comment that had
never been ratcheted.
Repos not yet at per-line coverage set
coverage-gate: floor, which renders inthe checks UI as
Coverage (FLOOR N% — migration debt, not the fleet gate)andemits a warning. The debt stays visible instead of looking like a passing gate.
Design points worth reviewing
rust-versionacross workspace members from
cargo metadata. Restating it in CI is how arepo ends up verifying 1.85 while promising 1.81 downstream. Verified against
browser-forensic, whose members declare both 1.80 and 1.85 — it derives 1.80.If no member declares one, the job fails loudly rather than guessing a floor it
would then claim to have verified.
fuzzcontaining aCargo.toml, at any depth. 79 fleet repos have a fuzz crate but only 70 keepit at
fuzz/; the other 9 usecore/fuzz,forensic/fuzz, orcrates/<member>/fuzz. A root-only check would skip those silently, and afuzz job that passes because it found nothing is worse than none at all.
Discovery was run against all 92 repos: exact match, zero mismatches.
.gitleaks.tomlis honoured — 60 repos ship one, allowlistingforensic fixtures that look like credentials.
docs(34 repos) andgeiger(19) are the obvious next consolidationcandidates, but they do not block adoption.
Validation
Python compiles.
(covered / uncovered / annotated exemption / bare marker / delimiter artifact /
unreadable source) — correct exit code and classification in each.
cargo llvm-covoutput (ntfs-forensic: 34files, 12,906
DArecords, 97 zero-hit). That caught a genuine bug my ownfixture had missed: the marker regex was anchored to
//and so dropped thetrailing block-comment form back into the failure set. Fixed and re-verified.
end, because the callee does not exist.
Action pins are behaviour-neutral
Every action is pinned to exactly what the fleet runs today, so this changes
one variable — where CI is defined — not two:
actions/checkout11bd71901bbe5b1630ceea73d27597364c9af683Swatinem/rust-cache9d47c6ad4b02e050fd481d890b2ea34778fd09d6taiki-e/install-action59012be0884e296ca2da49b530610e72c49039adAn earlier revision pinned
checkoutat v7.0.1 andrust-cacheat v2.9.1. Bothwere reverted — a version bump hidden inside a 91-repo consolidation is
unreviewable, and when CI breaks nobody can tell whether the migration or the
upgrade caused it.
rust-cacheis now the genuine v2.7.8, the version 56repos already believe they run, so this fixes the provenance label without
changing behaviour. Upgrades become a separate reviewable PR against one file.
cargo denyargument order--configbelongs to thechecksubcommand;--all-features,--excludeand
--targetare global options that precede it. Both forms were runagainst cargo-deny 0.19.0:
The workflow emits
cargo deny [globals] check --config <shared> all. Adeny-argsinput carries the three genuinely per-repo settings a shared configcannot hold:
--all-features(4 repos — kept out of the shared file because atfleet scope it pulls optional GPL-2.0/GPL-3.0 crates like
lzo1xandnoallocinto the graph),
--excludefor*-fuzzmembers (10 repos), and--target(winevt-forensic only, which per
cargo deny --help"overrides the top-leveltargets = []configuration value").